analysis: report rule state altered by other rule - v2 #12311
Conversation
Flowbits can make a rule such as a packet rule be treated as a stateful rule, without actually changing the rule type. Add a flag to allow reporting such cases via engine analysis. Task OISF#7456
jufajardini
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12311 +/- ##
=========================================
Coverage ? 83.25%
=========================================
Files ? 912
Lines ? 257638
Branches ? 0
=========================================
Hits ? 214508
Misses ? 43130
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 24033 |
|
|
||
| /* inter-signature state dependency */ | ||
| bool is_rule_state_dependant; | ||
| uint32_t rule_state_dependant_id; | ||
| uint32_t rule_state_variable_idx; |
There was a problem hiding this comment.
Can we get this info to analyzer w/o inflating this struct so much?
There was a problem hiding this comment.
I think this is fine. This is an init-time only helper struct
| @@ -1047,6 +1047,16 @@ void EngineAnalysisRules2(const DetectEngineCtx *de_ctx, const Signature *s) | |||
| break; | |||
| } | |||
|
|
|||
| if (s->init_data->is_rule_state_dependant) { | |||
| jb_open_object(ctx.js, "rule_state_dependant"); | |||
| jb_set_uint(ctx.js, "rule_depends_on_sid", s->init_data->rule_state_dependant_id); | |||
There was a problem hiding this comment.
what if there are multiple dependencies?
| if (s->init_data->is_rule_state_dependant) { | ||
| jb_open_object(ctx.js, "rule_state_dependant"); | ||
| jb_set_uint(ctx.js, "rule_depends_on_sid", s->init_data->rule_state_dependant_id); | ||
| jb_set_string(ctx.js, "rule_depends_on_flowbit", |
There was a problem hiding this comment.
can this just be flowbit? Seems the key is a bit redundant with the object name.
Also, as Shivani is asking: this is 1-N essentially, can we list them all? I'm changing my mind a bit on this. I think being complete is probably best.
So
"rule_state_dependant": {
"sids": [ 1901, 124, 666 ],
"flowbits": [ "fb2", "abc" ],
},
Flowbits can make a rule such as a packet rule be treated as a stateful rule, without actually changing the rule type.
Add a flag to allow reporting such cases via engine analysis.
Task #7456
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/7456
Previous PR: #12286
Output samples:
flowbits:setrule{ "raw": "alert http any any -> any any (msg:\"Setting flowbit fb2, app_tx rule\";http.uri;content:\"something\";flowbits:set,fb2;sid:1901;)", "id": 1901, "gid": 1, "rev": 0, "msg": "Setting flowbit fb2, app_tx rule", "app_proto": "http_any", "requirements": [ "flow" ], "type": "app_tx", "rule_state_dependant": false, }flowbits:issetrule{ "raw": "alert ip any any -> any any (msg:\"Is-Setting flowbit fb2, pkt rule\";flowbits:isset,fb2;sid:1904;)", "id": 1904, "gid": 1, "rev": 0, "msg": "Is-Setting flowbit fb2, pkt rule", "app_proto": "unknown", "requirements": [ "flow" ], "type": "pkt", "rule_state_dependant": { "rule_depends_on_sid": 1901, "rule_depends_on_flowbit": "fb2" }, }Describe changes:
init_datamostly generic, in case there are other scenarios where this could happen - but at least when it's time to report this via theengine-analysis, we need the info to fetch the variable namesetcommand that does that, from what I could understandProvide values to any of the below to override the defaults.
SV_BRANCH=OISF/suricata-verify#2201